fix: reject unsupported net-info output formats#3360
Conversation
Signed-off-by: haoshengzhen <haoshengzhen@outlook.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesNetInfoCmd Output Format Validation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/cmd/p2p.go (1)
44-46: ⚡ Quick winWrap flag-read errors with context.
Line 45 returns the raw error; add context so failures are diagnosable from CLI output/logs.
Proposed patch
outputFormat, err := cmd.Flags().GetString(flagOutput) if err != nil { - return err + return fmt.Errorf("read --%s flag: %w", flagOutput, err) }As per coding guidelines, "Wrap errors with context using fmt.Errorf in Go code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/cmd/p2p.go` around lines 44 - 46, In the error handling block around line 45 of the p2p.go command file, the raw error is being returned directly without any context information. Replace the bare return statement with fmt.Errorf to wrap the error and include a descriptive message about what operation failed (e.g., reading flags, parsing configuration, etc.). This ensures that when the error is displayed in CLI output or logs, it provides diagnostic context to help users understand what went wrong.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/cmd/p2p.go`:
- Around line 44-46: In the error handling block around line 45 of the p2p.go
command file, the raw error is being returned directly without any context
information. Replace the bare return statement with fmt.Errorf to wrap the error
and include a descriptive message about what operation failed (e.g., reading
flags, parsing configuration, etc.). This ensures that when the error is
displayed in CLI output or logs, it provides diagnostic context to help users
understand what went wrong.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9dc404a-3df4-4245-9bd2-63adf29e1831
📒 Files selected for processing (2)
pkg/cmd/p2p.gopkg/cmd/p2p_test.go
|
can you add a changelog? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3360 +/- ##
==========================================
- Coverage 60.97% 60.95% -0.02%
==========================================
Files 129 129
Lines 14111 14115 +4
==========================================
Hits 8604 8604
- Misses 4556 4558 +2
- Partials 951 953 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: haoshengzhen <haoshengzhen@outlook.com>
Thank you for your approval. Added a changelog entry under Please review it again. @julienrbrt |
Overview
This PR adds validation for the
net-info --outputflag.Previously,
net-infoonly handledjsonspecially and silently fell back to text output for any other value. For example,--output yamlwould still run the RPC requests and print text output, which is surprising for users and makes invalid CLI input harder to catch.The command now accepts only
textandjson, and returns a clear error for unsupported output formats before making RPC calls.Testing performed:
go test ./pkg/cmd -count=1gofmtgit diff --checkSummary by CodeRabbit
net-infocommand now properly validates the--outputflag and rejects unsupported output formats with a descriptive error message. Supported formats aretextandjson. Invalid format requests now fail immediately with clear feedback.--outputvalues (e.g.,yaml) are rejected.net-info --outputvalidation fix.